-
Notifications
You must be signed in to change notification settings - Fork 1
feat: js local resolver #32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
clientKey:string, | ||
clientId:string, | ||
clientSecret:string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clientKey:string, | |
clientId:string, | |
clientSecret:string, | |
flagClientSecret:string, | |
apiClientId:string, | |
apiClientSecret:string, |
openfeature-provider/js/src/lease.ts
Outdated
|
||
export type CachedProvider<T> = () => Promise<T> | ||
|
||
export function leaseFactory<T>(renewer:() => Promise<Lease<T>>, marginMs = 5000):CachedProvider<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do a background update before lease expires. Handle errors with retries.
openfeature-provider/js/src/lease.ts
Outdated
let current:Lease<T> | null = null; | ||
return async () => { | ||
if(!current || isExpired(current)) { | ||
const [value, expiry] = await renewer(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If renewer fails we MUST AT LEAST clear current value.
status: ProviderStatus = ProviderStatus.NOT_READY; | ||
|
||
private tokenProvider:CachedProvider<string>; | ||
private stateUriProvider:CachedProvider<{ signedUri:string, account:string }>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skip caching of uri!
import { CachedProvider, Lease, leaseFactory } from './lease'; | ||
import { ResolveReason } from './proto/api'; | ||
|
||
const DEFAULT_STATE_INTERVAL = 10_000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const DEFAULT_STATE_INTERVAL = 10_000; | |
const DEFAULT_STATE_INTERVAL = 30_000; |
return key in obj; | ||
} | ||
|
||
function valueMatchesSchema<T>(value:unknown, schema:T): value is T { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to something with assignsTo.
declare module "*.wasm" { | ||
const content: string; | ||
export default content; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
// private transferResponseSuccess<T>(value: T, codec: Codec<T>): number { | ||
// const data = codec.encode(value).finish(); | ||
// return this.transfer({ data }, Response); | ||
// } | ||
// private transferResponseError<T>(error: string): number { | ||
// return this.transfer({ error }, Response); | ||
// } | ||
|
||
// private consumeRequest<T>(ptr: number, codec: Codec<T>): T { | ||
// const req: Request = this.consume(ptr, Request); | ||
// return codec.decode(req.data); | ||
// } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
@@ -0,0 +1 @@ | |||
# js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ask AI to fix!
const { | ||
CONFIDENCE_API_CLIENT_ID, | ||
CONFIDENCE_API_CLIENT_SECRET, | ||
} = requireEnv('CONFIDENCE_API_CLIENT_ID', 'CONFIDENCE_API_CLIENT_SECRET'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just noting that these need to be added to github secrets once we want to run this in GH workflows.
return resp.json(); | ||
} | ||
|
||
private static convertReson(reason:ResolveReason):ResolutionReason { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spelling
private static convertReson(reason:ResolveReason):ResolutionReason { | |
private static convertReason(reason:ResolveReason):ResolutionReason { |
const onSuccess = async (resp:Response) => { | ||
attempts++; | ||
const { status, statusText } = resp; | ||
if(status !== 408 && status !== 429 && status < 500 || attempts >= maxAttempts) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure about this one... AI says:
fetch.ts:53 - Logic bug: The retry condition status !== 408 && status !== 429 && status < 500 has incorrect precedence. Should be (status !== 408 && status !== 429 && status < 500) or the current logic will retry on all 2xx/3xx responses.
async function bodyRepeater<T extends BodyInit>(body:BodyInit | null | undefined): Promise<() => T> { | ||
if(body instanceof ReadableStream) { | ||
const blob = await new Response(body).blob(); | ||
return () => blob.stream() as T | ||
} | ||
return () => body as T; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AI is suspicious of this function:
fetch.ts:176-182 - Type safety issue: bodyRepeater returns () => T but the generic constraint doesn't guarantee the return type matches, especially for ReadableStream conversion to Blob.
No description provided.